Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autodetect Docs V1 #3038

Merged
merged 21 commits into from
Nov 26, 2024
Merged

Autodetect Docs V1 #3038

merged 21 commits into from
Nov 26, 2024

Conversation

RomneyDa
Copy link
Collaborator

@RomneyDa RomneyDa commented Nov 22, 2024

Description

  • Searches for package files in repo
  • Parses packages from package files
  • Collects docs links from registries
  • Shows suggestions to user with relevant warnings, etc.
  • Indexing is immediate and shows in peek on form
  • Works for python and node currently

I should update the docs (the docs docs) before merging

Checklist

  • The relevant docs, if any, have been updated or created

Screenshots

image image

Copy link

netlify bot commented Nov 22, 2024

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit f7ce697
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/67452628874d5800082edfbe

Copy link
Collaborator

@Patrick-Erichsen Patrick-Erichsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Everything worked perfect for me out of the box. Just some UI nitpicks below, nothing that I think is preventing us from shipping so pick/choose what you think is worthwhile

  • I think we can remove the "FileIcon". Lot of color, takes up horizontal space, and is potentially a bit confusing, e.g. a given package.json dep may or may not be also written in TS
  • I think we should just use a lightgray PlusIcon for the leftmost "Add" action. The PlusCircleIcon is hard to read on small fonts in HeroIcons, and when it's the triangle it's unclear that you can still add it. Maybe warning goes alongside the URL somehow
  • "Done" and "Go" are a bit confusing to me. Maybe "Go" becomes "Add", and "Done" becomes "Close"? It's also a bit unclear to me whether those buttons are supposed to be associated to the entire dialog, or just the form
  • Feels like the title/start url needs a title of some sort

Got this when I didn't have a package.json
Screenshot 2024-11-22 at 9 17 43 AM

I think the spinners are a bit noisy with multiple loaders. I'd lean towards just removing
Screenshot 2024-11-22 at 10 56 21 AM

"No docs link found" isn't legible on dark mode unless the row is hovered
Screenshot 2024-11-22 at 10 26 25 AM

We have the horizontal space, so I think we could turn this into an "+ Add doc" button to make this more prominent
Screenshot 2024-11-22 at 10 50 02 AM

@RomneyDa
Copy link
Collaborator Author

@Patrick-Erichsen agree with/implemented all those changes

image image

@Patrick-Erichsen
Copy link
Collaborator

Beautiful! Can't approve on mobile I guess but LGTM 🫡

@RomneyDa RomneyDa requested a review from sestinj November 25, 2024 20:21
@RomneyDa
Copy link
Collaborator Author

Fixes #2715

@sestinj sestinj merged commit d695bc9 into main Nov 26, 2024
5 checks passed
@sestinj sestinj deleted the dallin/suggested-docs branch November 26, 2024 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants